-
-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow different field to be used as page label in list of pages #1122
Allow different field to be used as page label in list of pages #1122
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I agree on this change - menu
is just used in the navigation (in the menu), while a page might also appear in other places, and the reason you might have "menu" different than the page title are purely presentation reasons, for example if the page title does not fit the layout.
That's my 2c, maybe other people have another opinion on the matter.
I agree with the case of long titles breaking the layout, but the more elegant solution there is to truncate it with CSS. |
So far I only care about the list of pages, where it looks to me like there is always plenty of space. I'm willing to invest into making it a config option for the admin plugin to use either But I will only invest that time when I have a clear signal from you, that this would be useful. I'm also open for other ideas on how to customize what gets displayed in the admin panel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think an option in the plugin that allows you configure how pages are displayed is the best option. PR for this is welcome
3b3cc0e
to
1007547
Compare
I added the option |
One more remark, I only tested this code inside the stable version and cherry picked into this branch, as I have not set up a development environment with latest beta versions yet. |
You really should clone and install from develop, as PRs are only relevant when developed and tested for code that will make it into the next release. Any conflict between stable and develop will propagate and have to be resolved before release, so using anything other than the develop-branch is counterproductive. |
The branch that used for this PR is based on develop, and taking a look at the changes contained in that branch shows it does not contain anything but adding the configuration and using it. But I'm going to create a dev environment, test the code in this one and report back. |
Also works for me with the dev setup using all develop branches (except admin plugin, of course). The only question open is the one I raised this morning:
|
I compared the two options and in my opinion the more readable/ easy to understand code is when the fallback is applied where the data is used. |
Any feedback from your side @rhukster or changes required before this can land? |
Accepted for next release! Thanks. |
From the documentation and my current understaning the
title
field is used display the page, and themenu
field is used for navigational purposes.When a content editor is navigating the pages in the admin plugin using the
menu
field also provides an easy option to give more useful information, e.g. in a long list of child pages.I'm happy to do this change in more places where this concern might be present, but I know to little about the overall structure of the admin plugin to search by myself.
This could of course also be a configurable option, but as
menu
falls back totitle
it shouldn't do to much harm.(I just had the idea of checking if the
title
is contained inmenu
and displaying both if not...)Any feedback is welcome.